fix(ack-pay): reject empty payment option identifiers - #183
Conversation
paymentOptionSchema accepted empty strings for id, currency, and recipient, and paymentRequestSchema accepted an empty request id, even though other fields such as amount already reject invalid values. Require a non-empty string for these fields in both the valibot and zod schemas.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (4)
Included review availability: Your plan provides up to 2 included reviews per hour; 1 remains after this review. WalkthroughThe payment schemas now reject empty strings for payment option identifiers, currency, recipient, and payment request identifiers in both Valibot and Zod. Parameterized tests cover these constraints, and a patch changeset documents the update. ChangesPayment schema validation
Estimated code review effort: 2 (Simple) | ~10 minutes Merge Risk: ⚪ Minimal · up to The change rejects empty payment-request and payment-option identifiers across both supported schemas, preventing malformed inputs without adding new runtime or deployment behavior. No actionable merge-blocking risk remains after normal checks and review. Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
Full details: Docstring CoverageExplanation No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 3 files. (1 skipped: 1 unsupported.) ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
1 participant
Problem
paymentOptionSchemaaccepted empty strings forid,currency, andrecipient, andpaymentRequestSchemaaccepted an empty requestid. Other fields already reject invalid values —amountmust be positive, and the zod schema rejects negativedecimals— so an all-empty option was not an intentional allowance, justv.string()/z.string()with no length check.A signed payment request token can carry those fields through to verification, so the schema is the gate for HTTP 402 bodies and anything that parses them with
verifyPaymentRequestToken.Fix
Introduce a shared
nonEmptyStringin both schema files and use it for the payment requestidand forid,currency, andrecipienton each payment option. Valibot and zod stay in sync, per the repo's dual-schema rule.Tests
Extended
schemas.test.tswith parity coverage: each schema rejects a payment option with an emptyid,currency, orrecipient, and rejects a payment request with an empty top-levelid.Patch changeset included for
@agentcommercekit/ack-pay.AI usage
Per AI_POLICY.md, Cursor did the work during a rescan of main: it measured the empty-string acceptance, wrote the schema change and parity tests, and added the changeset. I read the diff before opening this.
Summary by CodeRabbit
Bug Fixes
Tests